Conversation
| #[derive(Debug)] | ||
| pub struct ClanSystem { | ||
| // TODO: add necessary fields | ||
| pub clans: Vec<Clan>, |
There was a problem hiding this comment.
Would using a std::collections::HashMap be better than Vec<Clan> here as clan ids are unique anyway?
| */ | ||
| pub fn get_clan_member_names(&self, clan_id: &str) -> Vec<String> { | ||
| unimplemented!(); | ||
| if let Some(clan) = self.clans.iter().find(|c| c.id == clan_id) { |
There was a problem hiding this comment.
Could using the hashmap help increase efficiency here as we might not have to iter through the whole list explicitely?
| */ | ||
| pub fn get_clan_member_count(&self, clan_id: &str) -> usize { | ||
| unimplemented!(); | ||
| if let Some(clan) = self.clans.iter().find(|c| c.id == clan_id) { |
There was a problem hiding this comment.
Hashmap could help improve brevity and efficiency here as well I think.
| } | ||
|
|
||
| pub fn add_member_to_clan(&mut self, clan_id: &str, crab_name: &str) { | ||
| if let Some(clan) = self.clans.iter_mut().find(|c| c.id == clan_id) { |
There was a problem hiding this comment.
Can we also maybe make this implemetation succint & efficient by use of hashmap and functions available for hashmap?
| .max_by_key(|clan| clan.members.len()) | ||
| .map(|clan| clan.id.clone()) | ||
| } | ||
|
|
There was a problem hiding this comment.
Could we add comment over this function to explain what it does to increase readability?
| fn get_crab_speed_by_name(&self, crab_name: &str) -> Result<u32, String> { | ||
| for crab in self.crabs() { | ||
| if crab.name() == crab_name { | ||
| return Ok(crab.speed()); |
There was a problem hiding this comment.
Can we remove the keyword return here for consistency in coding style?
| } | ||
| } | ||
|
|
||
| fn calculate_average_speed(&self, clan_id: &str) -> Result<u32, String> { |
There was a problem hiding this comment.
Good job of breaking a big function into smaller logical functions! Could we add comment over this function to explain what it does to increase readability?
| } | ||
| } | ||
|
|
||
| fn get_crab_speed_by_name(&self, crab_name: &str) -> Result<u32, String> { |
There was a problem hiding this comment.
Good job of breaking a big function into smaller logical functions! Could we add comment over this function to explain what it does to increase readability?
|
|
||
| #[derive(Debug)] | ||
| pub struct Beach { | ||
| // TODO: Declare the fields of the Beach struct here. |
There was a problem hiding this comment.
Could we remove the TODO: statements here if they are no longer needed?
| } | ||
|
|
||
| fn calculate_average_speed(&self, clan_id: &str) -> Result<u32, String> { | ||
| let clan_system = self.get_clan_system(); // Assuming you have a method to get the clan system |
There was a problem hiding this comment.
Can we remove this comment as we do have a method to get clan system in the code?
|
Overall LGTM, also all given test cases passed when I checked. Please have a look at the comments and let me know what you think! |
2 participants
Implement codes following Assignment 1 requirements.